-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add option to configure GCP logging extension's logging target #455
Conversation
...e/src/main/java/io/quarkiverse/googlecloudservices/logging/runtime/LoggingConfiguration.java
Show resolved
Hide resolved
* Enable or disable default Quarkus console logging. | ||
*/ | ||
@ConfigItem | ||
public boolean enableConsoleLogging; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This config property is not used anywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is used, inside LoggingBuildSteps
, where we configure if we want to disable Quarkus console logging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but it is not used in runtime code so it seems useless, the logTarget is enough IMHO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not useless because it disables console logging with which we avoid duplicated logs. This GCP extension is currently registering another log handler, which causes us to have 2 logging handlers and 2 log outputs. With this option, we have the option to disable this by default so that user does not have to disable them manually (but they still have the option to keep console logging if they want).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just understood what you did!
If someone want to disable console login, he can use the quarkus.log.console.enable
property, I don't see the point of providing another config property that will set this one for him.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure I just thought it would be nice to have this disabled by default, but ok. Maybe would be beneficial to put this somewhere in the docs so users are not confused if they get doubled logs :)
* Enable or disable default Quarkus console logging. | ||
*/ | ||
@ConfigItem | ||
public boolean enableConsoleLogging; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but it is not used in runtime code so it seems useless, the logTarget is enough IMHO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
This resolves #453.
I introduced two new configuration options:
enableConsoleLogging
: with this user can enable or disable default Quarkus console logging. By default, this is disabled so that we prevent duplicated logs.logTarget
: with this user can configure the target of the logs: stdour, stderr, or Google Cloud API. It follows a similar approach as in the official Google Java Logging library (we even use their enums to avoid having to add additional boilerplate code).With this user has the option to configure logging targets as they wish. They can output logs to Cloud Logging API and still use Quarkus console logging or they disable console logging in case they do not need them or they want to avoid duplication of logs (if they host their app on Google Cloud Platform).
Testing: using Quarkus getting-started sample on local and on GCP environment.